-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate to cloud framework #1080
Conversation
950483c
to
5b21744
Compare
flags = "f", | ||
perms = Permissions.START) | ||
@CommandMethod("recycle|rematch [duration]") | ||
@CommandDescription("Reload (cycle to) the current map") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always wanted to see if we can make these translatable. Do you think it’s possible with cloud?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the framework is flexible enough that i'm certain it can be done, but it requires using our own annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want them to be globally translatable or per sender? There are different approaches depending on what you need :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We handle all translations in PGM per-sender, via adventure. We have our own systems for that here. Ideally we'd simply make the descriptions be an adventure component created from a translatable with a key specified in the @CommandDescription
annotation, then in MinecraftHelp make sure it uses the components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With builders you can set a component in the command meta directly (see MinecraftExtrasMetaKeys), with annotations you probably need a command builder modifier to retrieve component instances by key or something. Can either use the CommandDescription itself to store the intermediary key, or could add your own annotation and meta value
This has moved over to ready for review. The migration is complete, although some things no longer work as they used to. There's an upstream PR for cloud to allow flag parsing in any location (Incendo/cloud#395) that is required for things like Besides that one issue, everything else is smoother than it used to, and map autocompletion has gotten alot of work, now very wild things auto-complete nicely and predictably. Things like |
f4dbb8d
to
e9b88f9
Compare
Am i understanding correctly that this closes #66 ? |
it does |
cae0d5a
to
06d84e1
Compare
Pushed a few changes, made parsers for PlayerClass & SettingKey. Setting keys will suggest & parse based on the previous argument for setting, so if you Additionally since Incendo/cloud#395 doesn't seem to be getting any progress, i'm considering making PGM run a cloud fork until the changes get upstreamed so that this PR doesn't keed to keep waitning. |
06d84e1
to
28dbaa8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pablete1234 If you resolve the conflict, this is good to merge.
Signed-off-by: Pablete1234 <[email protected]>
Signed-off-by: Pablete1234 <[email protected]>
Signed-off-by: Pablete1234 <[email protected]>
Signed-off-by: Pablete1234 <[email protected]>
Signed-off-by: Pablete1234 <[email protected]>
Signed-off-by: Pablete1234 <[email protected]>
28dbaa8
to
8067f4c
Compare
This is still WIP (a couple command classes still comented out) but already very functional and usable version of pgm using the cloud command framework (https://github.com/Incendo/cloud).
The main if not only regression is that flags are not (yet) supported in the middle of the command, so they expect to always be at the end. However i've already made a proof-of-concept fix for this in Incendo/cloud@430beb7, so i'm hoping a fix can either be upstream soon, or we'll have to run a fork until it gets merged.
The benefits are several, from better autocompletion, to support for "MatchPlayer" as both injection (context/sender) and argument (ie: targeting someone else), cleaner commands, better tab-completion, and greedy strings (like /g, /msg etc) no longer replace quotes or prevent
-f
being typed as it's detected as a flag.Example of greedy text being auto completed by fuzzy-matching (note how entered text is gold not golden):
javaw_9eu7wkrr2M.mp4